Conversation
📝 WalkthroughWalkthroughThis pull request refines the Changes
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/util.ts (2)
751-753: Consider extracting the normalization logic into a helper function.The pattern of formatting values with
toFixed(normalizedDecimalCount)before parsing withethers.utils.parseUnitsis repeated three times. Consider extracting this into a helper function to improve code maintainability and readability.+ function normalizeTokenValue(value: number | string, normalizedDecimalCount: number): string { + return Number(value).toFixed(normalizedDecimalCount); + } + // Normalize prices to account for different decimal counts between tokens. // This ensures calculations are consistent and prevents issues with scientific notation // that could arise from small price values or different token decimals. const normalizedDecimalCount = Math.max(fromToken.decimals, toToken.decimals) - const fromTokenPriceBN = ethers.utils.parseUnits(fromTokenPrice.toFixed(normalizedDecimalCount), normalizedDecimalCount) - const toTokenPriceBN = ethers.utils.parseUnits(toTokenPrice.toFixed(normalizedDecimalCount), normalizedDecimalCount) - const toAmountBN = ethers.utils.parseUnits(Number(toAmount).toFixed(normalizedDecimalCount), normalizedDecimalCount) + const fromTokenPriceBN = ethers.utils.parseUnits(normalizeTokenValue(fromTokenPrice, normalizedDecimalCount), normalizedDecimalCount) + const toTokenPriceBN = ethers.utils.parseUnits(normalizeTokenValue(toTokenPrice, normalizedDecimalCount), normalizedDecimalCount) + const toAmountBN = ethers.utils.parseUnits(normalizeTokenValue(toAmount, normalizedDecimalCount), normalizedDecimalCount)🧰 Tools
🪛 ESLint
[error] 751-751: Replace
fromTokenPrice.toFixed(normalizedDecimalCount),·normalizedDecimalCountwith⏎↹↹↹fromTokenPrice.toFixed(normalizedDecimalCount),⏎↹↹↹normalizedDecimalCount⏎↹↹(prettier/prettier)
[error] 752-752: Replace
toTokenPrice.toFixed(normalizedDecimalCount),·normalizedDecimalCountwith⏎↹↹↹toTokenPrice.toFixed(normalizedDecimalCount),⏎↹↹↹normalizedDecimalCount⏎↹↹(prettier/prettier)
[error] 753-753: Replace
Number(toAmount).toFixed(normalizedDecimalCount),·normalizedDecimalCountwith⏎↹↹↹Number(toAmount).toFixed(normalizedDecimalCount),⏎↹↹↹normalizedDecimalCount⏎↹↹(prettier/prettier)
751-753: Address formatting issues flagged by ESLint.ESLint has flagged some formatting issues with the fixed lines. While these don't affect functionality, they should be addressed for code consistency.
- const fromTokenPriceBN = ethers.utils.parseUnits(fromTokenPrice.toFixed(normalizedDecimalCount), normalizedDecimalCount) - const toTokenPriceBN = ethers.utils.parseUnits(toTokenPrice.toFixed(normalizedDecimalCount), normalizedDecimalCount) - const toAmountBN = ethers.utils.parseUnits(Number(toAmount).toFixed(normalizedDecimalCount), normalizedDecimalCount) + const fromTokenPriceBN = ethers.utils.parseUnits( + fromTokenPrice.toFixed(normalizedDecimalCount), + normalizedDecimalCount + ) + const toTokenPriceBN = ethers.utils.parseUnits( + toTokenPrice.toFixed(normalizedDecimalCount), + normalizedDecimalCount + ) + const toAmountBN = ethers.utils.parseUnits( + Number(toAmount).toFixed(normalizedDecimalCount), + normalizedDecimalCount + )🧰 Tools
🪛 ESLint
[error] 751-751: Replace
fromTokenPrice.toFixed(normalizedDecimalCount),·normalizedDecimalCountwith⏎↹↹↹fromTokenPrice.toFixed(normalizedDecimalCount),⏎↹↹↹normalizedDecimalCount⏎↹↹(prettier/prettier)
[error] 752-752: Replace
toTokenPrice.toFixed(normalizedDecimalCount),·normalizedDecimalCountwith⏎↹↹↹toTokenPrice.toFixed(normalizedDecimalCount),⏎↹↹↹normalizedDecimalCount⏎↹↹(prettier/prettier)
[error] 753-753: Replace
Number(toAmount).toFixed(normalizedDecimalCount),·normalizedDecimalCountwith⏎↹↹↹Number(toAmount).toFixed(normalizedDecimalCount),⏎↹↹↹normalizedDecimalCount⏎↹↹(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/util.ts(1 hunks)
🧰 Additional context used
🪛 ESLint
src/util.ts
[error] 751-751: Replace fromTokenPrice.toFixed(normalizedDecimalCount),·normalizedDecimalCount with ⏎↹↹↹fromTokenPrice.toFixed(normalizedDecimalCount),⏎↹↹↹normalizedDecimalCount⏎↹↹
(prettier/prettier)
[error] 752-752: Replace toTokenPrice.toFixed(normalizedDecimalCount),·normalizedDecimalCount with ⏎↹↹↹toTokenPrice.toFixed(normalizedDecimalCount),⏎↹↹↹normalizedDecimalCount⏎↹↹
(prettier/prettier)
[error] 753-753: Replace Number(toAmount).toFixed(normalizedDecimalCount),·normalizedDecimalCount with ⏎↹↹↹Number(toAmount).toFixed(normalizedDecimalCount),⏎↹↹↹normalizedDecimalCount⏎↹↹
(prettier/prettier)
🔇 Additional comments (2)
src/util.ts (2)
751-753: Great fix for handling token prices with more decimals than the token.The change from
toString()totoFixed(normalizedDecimalCount)properly formats numeric values to a fixed number of decimal places before parsing them withethers.utils.parseUnits. This prevents potential issues with scientific notation, floating-point precision, and ensures consistent handling of token amounts when dealing with cross-chain transactions.Adding
Number()wrapper fortoAmountis also important since it ensures the string value is properly converted to a number before applyingtoFixed().🧰 Tools
🪛 ESLint
[error] 751-751: Replace
fromTokenPrice.toFixed(normalizedDecimalCount),·normalizedDecimalCountwith⏎↹↹↹fromTokenPrice.toFixed(normalizedDecimalCount),⏎↹↹↹normalizedDecimalCount⏎↹↹(prettier/prettier)
[error] 752-752: Replace
toTokenPrice.toFixed(normalizedDecimalCount),·normalizedDecimalCountwith⏎↹↹↹toTokenPrice.toFixed(normalizedDecimalCount),⏎↹↹↹normalizedDecimalCount⏎↹↹(prettier/prettier)
[error] 753-753: Replace
Number(toAmount).toFixed(normalizedDecimalCount),·normalizedDecimalCountwith⏎↹↹↹Number(toAmount).toFixed(normalizedDecimalCount),⏎↹↹↹normalizedDecimalCount⏎↹↹(prettier/prettier)
751-753:Details
❓ Verification inconclusive
Verify the fix by testing with extreme decimal values.
To ensure the fix completely resolves the issue, test with extreme cases like:
- A token price with many more decimal places than the token (e.g., 18 vs 6)
- Very small token prices (e.g., 0.0000000001)
- Token amount values that are strings vs numbers
These edge cases will confirm the robustness of the fix.
🏁 Script executed:
#!/bin/bash # Check for any related tests that might verify this functionality echo "Searching for tests that might verify prepareXchainFromAmountCalculation function..." rg -l "prepareXchainFromAmountCalculation" --type tsLength of output: 266
Ensure Robustness with Extreme Decimal Cases and Diverse Input Types
The current tests (located in
test/unit/util.test.ts) already execute functionality related toprepareXchainFromAmountCalculationinsrc/util.ts. However, please verify that these tests explicitly cover:
- Token Price with Excess Decimal Places: Use cases where the token price has significantly more decimals (e.g., 18) than the token's supported decimals (e.g., 6).
- Very Small Token Prices: Values such as
0.0000000001to check for precision issues.- Diverse Input Types for Token Amounts: Ensure the function correctly handles both numeric and string representations of token amounts.
If these scenarios are not explicitly covered yet, please add relevant tests to fully validate the robustness of the conversion fix.
🧰 Tools
🪛 ESLint
[error] 751-751: Replace
fromTokenPrice.toFixed(normalizedDecimalCount),·normalizedDecimalCountwith⏎↹↹↹fromTokenPrice.toFixed(normalizedDecimalCount),⏎↹↹↹normalizedDecimalCount⏎↹↹(prettier/prettier)
[error] 752-752: Replace
toTokenPrice.toFixed(normalizedDecimalCount),·normalizedDecimalCountwith⏎↹↹↹toTokenPrice.toFixed(normalizedDecimalCount),⏎↹↹↹normalizedDecimalCount⏎↹↹(prettier/prettier)
[error] 753-753: Replace
Number(toAmount).toFixed(normalizedDecimalCount),·normalizedDecimalCountwith⏎↹↹↹Number(toAmount).toFixed(normalizedDecimalCount),⏎↹↹↹normalizedDecimalCount⏎↹↹(prettier/prettier)
2 participants
No description provided.